I don't think this is the right solution. We should be reusing out existing MANIFEST.SKIP infrastructure and conventions instead of reinventing a wheel.

I don't think this is the right solution. We should be reusing out existing MANIFEST.SKIP infrastructure and conventions instead of reinventing a wheel.

Essentially what we should do is just

use ExtUtils::Manifest;
my $skip = maniskip;
chomp(my @git-files = `git ls-files`);
my @files = grep !$skip->($_), @git-files;

I personally would prefer a proper
MANIFEST file for a number of reasons including a proper description
of each file

I do agree with that

Ok, I have removed the "Work In Progress" flag, and I am ready to debate this now. Thank you for your patience. :-)

Please review the PR message (same as the commit message), it explains why I don't think that this should be a MANIFEST.skip functionality. The strongest argument IMO is A1, which I will paraphrase here:

Quite a bit of our build tooling expects to be able to read the MANIFEST file and then do stuff based on the contents. MANIFEST.skip is designed to exclude things from the creation or updates of a MANIFEST, usually via "make manifest" in a normal CPAN dist (which our Makefile does not support!). But we want to use these lists of files to do further processing, so a list of regexes and file globs doesn't really cut it. Especially in terms of ease of conversion to use the new files having two manifest like files is much easier to deal with, all the places we load the current MANIFEST can be changed to load both and operate on the union. Doing the same thing with skip file would IMO take a lot more effort for minimal benefit. In other words while it might seem like using maniskip() type functionality is suited to the task, IMO it really isn't.

There are other IMO good reasons this should not "just" reuse the maniskip functionality, and I have listed them all in the commit message, but the fact that we want to operate and process the list of files is the strongest.

This does a bunch of tooling changes, a bunch of changes to what files are shipped in a tarball, a few additions to the descriptions of files in the MANIFEST and updating a piece of documentation. Those should at the very least be split into 4 separate commits (and possibly the tooling changes could be split further)

Various parts of our build and test process expect to read the MANIFEST file and then do things based on the entries contained within.

What are these things, that might be adversely affected by listing some Porting/* files in MANIFEST.SKIP?

I think that MANIFEST.skip style files of exclusion regexens and globs are error prone and easy to mess up, for instance by excluding far more than you had intended to

MANIFEST.SKIP can also handle literal entries, without wildcards, if it is more convenient to list every file out longhand.

What are these things, that might be adversely affected by listing some Porting/* files in MANIFEST.SKIP?

As far as I can tell that's only t/porting/exec-bit.t, except it's wrong. Porting/exec-bit.txt is what we use to set +x bits when building a release, so files that aren't part of a release shouldn't be listed in it. So it doesn't need the skip-list after all.

Q. Why isn't this using MANIFEST.skip style functionality?

A1. Various parts of our build and test process expect to read the MANIFEST file and then do things based on the entries contained within. Eg, run tests, or extract data, or compare the file list to content in another file. Those parts of our build process would break if we used a skip style list of regexen. So it would be more work to teach them to deal with such a file, assuming it was actually doable - given the additional work I have not considered it deeply. On the other hand teaching that logic to simply read two files was and is easy.

That seems to assume that using MANIFEST.SKIP means not using MANIFEST. I don't think anyone has suggested doing away with the latter. We would use MANIFEST.SKIP for two reasons: regenerating the MANIFEST, and testing it for correctness. Anything else would use MANIFEST as it does now.

A2. I think each file we have in the repo should have a description. This patch currently doesn't provide a description for each, but it does for many, especially those migrated from MANIFEST.

Descriptions can be useful, but I really don't see the value of «.gitignore rules for the Digest-SHA cpan distribution» for dozens of cpan distributions.

A3. I think that MANIFEST.skip style files of exclusion regexens and globs are error prone and easy to mess up, for instance by excluding far more than you had intended to. They can also be annoying to get right, obviously not impossible, but sometimes annoying. Explicitly listing everything is easy in every way, especially to mechanize.

We have tests. We are checking for correctness. I don't see how this would suddenly become error-prone. Again, this seems to assume that there wouldn't be a MANIFEST file, which is not how MANIFEST.SKIP is typically used.

A4. I would like to be able to move verbatim entries from our existing MANIFEST into the new Porting/MANIFEST.dev, description and all. MANIFEST.skip style files do not support descriptions except as comments as far as I recall. That would have meant munging the data from MANIFEST during the move process which would be annoying.

As far as I know, MANIFEST.SKIP does support descriptions (though it isn't well-documented).

t/porting/readme.t and t/porting/readme.t. Also some of our test logic uses MANIFEST to get the list of tests to run. So far we havent moved any t files into Porting/MANIFEST.dev but it seems reasonable that we will. I expect @atoomic will be pushing some patches on top of this branch to do things like that.

You are welcome to migrate this all to use MANIFEST.skip instead, I don't think its worth the effort.

I found another 7 places where we use the manifest file to drive testing or other processing. I really don't get how you propose to make this work with MANIFEST and MANIFEST.SKIP. The idea of MANIFEST.SKIP is to designate files that should NOT be included in the manifest. So for the places where we want to have a way to get a list of the things we would have to abuse the MANIFEST.SKIP functionality to do so, or rewrite code to walk the disk or something, which brings additional side-issues with it. So likely we would have to read two file formats. The work I just did was mostly mechanical, to make the code read two files and not one.

Nothing i said implied that using MANIFEST.SKIP would NOT involve using MANIFEST, it implies that MANIFEST would be missing files, and thus in the places where we want to read a manifest of ALL the files, including those not part of the final tarball, that we would have to read that list from two totally differently formatted files. Your way would involve more complex processing, and likely more complex changes. I didn't want to get into analyzing whether choices made by previous devs in those files was sensible, for instance your point about the execute-bit test, somebody decided it was a good idea to have files in Porting check that the files were listed there. I don't know if that was a good decision or not, ill leave that to someone else.

Regarding your comment about descriptions for .gitignore, sure, that doesn't add that much value, I just did that for completeness and to establish the trend that things should be described. And in one of the comments I included some information about how to get documentation on .gitignore, which I think is useful, likewise on .gitattributes, and I described .git_patch, and the other files in the list do have useful descriptions. So I don't really think your point about .gitignore is very helpful to the discussion. You seem to think this discussion is about having a way to specify the things we ignore in certain places. IMO that is not the point, the point to me is how do we have our cake, a fully documented MANIFEST like database of our files, and still be able to distinguish between files that should be in the tarball and final release and those that shouldn't, and how and where do we authoritatively document and list the files. The fact that previously we listed almost everything in MANIFEST and then in a few places where tested that MANIFEST listed everything we filtered out a few special cases isn't in my eyes representative of the core problem I want to solve here, if it was then I admit MANIFEST.SKIP would be the way to go. In fact I could easily imagine my plan coupling with the use of a MANIFEST.SKIP file as well, for the few places where we want to look at the union of all our files and then exclude some files based on regex. There are two such examples in the code I just pushed. But that wouldn't make MANIFEST.dev go away, it would add a third data file. Personally i don't think there is evidence /right now/ that we should use such a complex a mechanism, the two cases are very specific. One of them might even totally redundant.

Maybe your proposal would "perfect", but IMO this is a good case of perfect is the enemy of good enough, this is good enough, it works, and it passes test, it is conceptually simple, and it scratches the itch that was expressed. Also, my plan has one big advantage over your plane, mine is already done. Maybe you want to put together a patch that does it your way to prove your point, but I don't think your plan is as good an idea as you do so I won't be doing it that way myself.

Also, my plan has one big advantage over your plane, mine is already done.

I can't overstate how annoyed I am at this statement, even if I was expecting it.

Not just because ignores the obvious and entirely valid third option of leaving things the way they are, but primarily because it is a rather hostile way of cooperation. You start with «But for now I'd rather not debate it with you. When the PR goes out of "work in progress" I will be happy to discuss more.», and then the next day you come up with this "I already wrote it so we might as well merge it".

Frankly, if you can't even pretend to be interested in actually listening to what other people are saying I'm not going to waste my time and energy on this discussion and would rather suggest for this ticket to be closed.

Maybe you want to put together a patch that does it your way to prove your point, but I don't think your plan is as good an idea as you do so I won't be doing it that way myself.

Sure: #19523

Also, if you're going to make fundamental changes to how tarballs are built, you probably want to double check if that tarball works. Right now it fails to build a tarball, but even if it didn't I am expecting it to fail some of those porting tests because of missing files.

Note: #19542 provides one suggestion to merge both ideas from #19513 and #19523